[SEC] restrict CORS to authorized extension IDs by RaoufGhrissi · Pull Request #581 · ActivityWatch/aw-server-rust

RaoufGhrissi · 2026-04-02T11:34:57Z

[SEC] restrict CORS to authorized extension IDs
Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction.

Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI.

We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration.

Dependent on: ActivityWatch/aw-webui#795

greptile-apps · 2026-04-02T11:54:06Z

Greptile Summary

This PR hardens CORS security by replacing the blanket moz-extension://.* wildcard with two explicit config flags (cors_allow_aw_chrome_extension defaulting to true, cors_allow_all_mozilla_extension defaulting to false) and adds a new /api/0/cors-config REST endpoint for viewing and persisting CORS settings via the Web UI. A companion fix in aw-datastore/src/worker.rs corrects a pre-existing bug where SetKeyValue and DeleteKeyValue commands failed to set self.commit = true, silently discarding all key-value writes across restarts.

Several issues raised in earlier review rounds have been resolved in this revision: serde_json::from_str now replaces fragile trim_matches parsing, cors_allow_all_mozilla_extension correctly defaults to false, the state.config mutex is updated after each cors_config_set call, and the testing-mode Chrome-extension glob is pushed to allowed_regex_origins instead of allowed_exact_origins.

Confidence Score: 5/5

Safe to merge — all P0/P1 issues from the previous review round are resolved; remaining findings are P2 style and improvement suggestions.

The significant defects identified in earlier rounds (wrong default for cors_allow_all_mozilla_extension, |= write-once boolean bug, state.config not updated after save, fragile trim_matches JSON parsing, testing-mode regex in wrong list) are all fixed. The datastore worker commit-flag bug is also correctly patched. Remaining findings are a validation inconsistency, missing test coverage, and a minor per-request filesystem read — none block merging.

aw-server/src/endpoints/cors_config.rs (exact-origin validation) and aw-server/tests/api.rs (no coverage for new endpoints)

Important Files Changed

Filename	Overview
aw-server/src/endpoints/cors_config.rs	New GET/POST endpoints for CORS config; regex patterns are validated before saving; in-memory config is updated after save so GET reflects changes immediately. Exact-origin validation rejects chrome-extension:// and moz-extension:// schemes, silently forcing extension origins into cors_regex only, inconsistent with config.toml.
aw-datastore/src/worker.rs	Critical bugfix: SetKeyValue and DeleteKeyValue now set self.commit = true, ensuring key-value changes are actually persisted across restarts. Previously all settings writes were silently dropped on shutdown.
aw-server/src/endpoints/cors.rs	Rewrites CORS origin construction to use explicit flags instead of hardcoded wildcards; testing-mode Chrome-extension glob is correctly in the regex list.
aw-server/src/config.rs	Adds cors_allow_aw_chrome_extension (default true) and cors_allow_all_mozilla_extension (default false) fields; create_config correctly loads DB-persisted values for fields absent from the TOML file using serde_json::from_str.
aw-server/tests/api.rs	Updated to include config in ServerState; no tests added for the new /api/0/cors-config endpoints.

Sequence Diagram

sequenceDiagram
    participant Client as Web UI / Client
    participant Server as aw-server
    participant Fairing as rocket_cors Fairing
    participant Config as state.config (Mutex)
    participant DB as Datastore (key-value)
    participant TOML as config.toml

    Note over Server: Startup
    Server->>TOML: read config, identify missing CORS fields
    Server->>DB: load missing fields from cors.* keys
    Server->>Config: initialise AWConfig
    Server->>Fairing: build_cors(config) — snapshot of origins

    Note over Client,Fairing: Runtime — read current settings
    Client->>Server: GET /api/0/cors-config
    Server->>TOML: get_config_path() — re-read file to find in_file fields
    Server->>Config: lock and read values
    Server-->>Client: CorsConfig { ..., needs_restart: true }

    Note over Client,Fairing: Runtime — save new settings
    Client->>Server: POST /api/0/cors-config
    Server->>DB: set_key_value(cors.*, ...) for non-file fields
    Server->>Config: lock and update in-memory values
    Server-->>Client: 200 OK
    Note over Fairing: Fairing NOT updated — restart required

_{Reviews (8): Last reviewed commit: "[SEC] restrict CORS to authorized extens..." | Re-trigger Greptile}

greptile-apps · 2026-04-02T11:54:13Z

aw-server/src/endpoints/cors.rs

+    let parse_cors_list = |key: &str| -> Vec<String> {
+       db.get_key_value(key)
+            .map(|s| {
+                s.trim_matches('"')
+                    .split(',')
+                    .map(|s| s.trim().to_string())
+                    .filter(|s| !s.is_empty())
+                    .collect()
+            })
+            .unwrap_or_default()
+    };


Fragile JSON string unwrapping via trim_matches

The settings API stores all values JSON-encoded (see settings.rs line 72: serde_json::to_string(&value.0)). So a string like "moz-extension://abc" is persisted in the database as "\"moz-extension://abc\"".

trim_matches('"') strips surrounding double-quote characters, which works for simple ASCII strings, but is not a correct JSON string deserializer. It will silently produce wrong results for values that contain escaped characters. It also won't handle the case where the user stores a JSON array (e.g., ["ext1", "ext2"]) instead of a comma-separated string.

The idiomatic fix is to use serde_json::from_str::<String>(&s):

let parse_cors_list = |key: &str| -> Vec<String> { db.get_key_value(key) .ok() .and_then(|s| serde_json::from_str::<String>(&s).ok()) .map(|s| { s.split(',') .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()) .collect() }) .unwrap_or_default() };

greptile-apps · 2026-04-02T11:54:14Z

aw-server/src/endpoints/cors.rs

+    let mut allowed_regex_origins = vec!["chrome-extension://nglaklhklhcoonedhgnpgddginnjdadi".to_string()];
+
+    let settings_origins = parse_cors_list("settings.cors_origins");
+    drop(db);
+
+    let all_origins = config.cors.iter().cloned().chain(settings_origins);
+    for origin in all_origins {
+        if origin.starts_with("http://") || origin.starts_with("https://") {
+            allowed_exact_origins.push(origin);
+        } else if origin.starts_with("chrome-extension://") || origin.starts_with("moz-extension://") {
+            allowed_regex_origins.push(origin);
+        } else {
+            log::warn!("Ignoring invalid CORS origin: '{}'", origin);
+        }
+    }


Firefox extension support is effectively broken for all existing users

The original code included the comment:

Every version of a mozilla extension has its own ID to avoid fingerprinting, so we unfortunately have to allow all extensions to have access to aw-server

This is a fundamental Firefox security property: each installation (and each update) of a Firefox extension gets a new random UUID as its origin (moz-extension://<uuid>). This means:

There is no single stable ID that a user can add to the allow-list for the ActivityWatch Firefox extension.

After every extension update, the old ID is invalidated and the user would need to find their new UUID and update the server settings.

Most users have no easy way to discover their current moz-extension:// UUID.

The PR removes moz-extension://.* without providing any workable alternative for Firefox users. Consider either:

Keeping moz-extension://.* as the default (preserving existing behavior) with clear documentation about the risk, or

Using a permanent addon ID via browser_specific_settings.gecko.id in manifest.json and documenting that users of unofficial builds need the wildcard.

aw-server/src/endpoints/cors.rs

ErikBjare · 2026-04-02T12:17:02Z

aw-server/src/endpoints/mod.rs

        config.address, config.port
    );
-    let cors = cors::cors(&config);
+    let cors = cors::cors(&config, &server_state.datastore);


Don't pass datastore here

RaoufGhrissi · 2026-04-02T15:18:39Z

aw-server/src/config.rs

+    #[serde(default = "default_true")]
+    pub allow_aw_chrome_extension_from_settings: bool,
+
+    #[serde(default = "default_false")]


i'm hesitating to set default value to True, so it doesn't impact the actual users

aw-server/src/config.rs

aw-server/src/endpoints/cors.rs

ErikBjare · 2026-04-02T15:38:05Z

aw-server/src/config.rs

+            cors_from_settings: default_cors(),
+            cors_regex_from_settings: default_cors(),
+            allow_aw_chrome_extension_from_settings: default_true(),
+            allow_all_mozilla_extension_from_settings: default_false(),


Not a fan of this many additional config keys. Should be prefixed consistently with cors_ such as cors_allow_aw_chrome_extension and cors_allow_all_mozilla_extension.

Curious why the _from_settings is passed along and not resolved/merged into the non-from_settings variants?

merged with the existing vars, and explained in the readme file

ErikBjare · 2026-04-02T15:40:58Z

aw-server/src/endpoints/mod.rs

+        if let Ok(raw) = db.get_key_value("settings.cors") {
+            config.cors_from_settings = parse_cors_list(raw);
+        }
+        if let Ok(raw) = db.get_key_value("settings.cors_regex") {
+            config.cors_regex_from_settings = parse_cors_list(raw);
+        }
+        if let Ok(raw) = db.get_key_value("settings.allow_aw_chrome_extension") {
+            config.allow_aw_chrome_extension_from_settings = parse_bool(raw);
+        }
+        if let Ok(raw) = db.get_key_value("settings.allow_all_mozilla_extension") {
+            config.allow_all_mozilla_extension_from_settings = parse_bool(raw);
+        }
+    }


I'm a little bit paranoid about letting web UI set CORS like that...

do you have another suggestion to let non-devs configure cors ?

aw-server/src/endpoints/cors.rs

aw-server/src/endpoints/mod.rs

ErikBjare · 2026-04-08T09:31:46Z

aw-server/src/endpoints/mod.rs

+        // Sync settings between Config file and Database.
+        // On the first run (when a key is missing in the DB), we seed the DB with the value from the config file.
+        // On subsequent runs, we always prefer the DB value (which might have been changed via the UI).


This seems problematic. It's not acceptable to have file-level config options being ignored by auto-set db-values. Makes the config file mostly useless/unreliable. Not really any "sync" happening here.

imo, order of settings/config precedence in server should be cli options -> env vars -> config file -> webui/api settings -> defaults.

One way to resolve this would be to make the webui config write directly to the config file, would effectively put them in sync (but still not a fan of exposing server config like this in api/webui tbh). But that might require a API endpoint different from /settings, maybe /config - but really unsure about this, just an idea.

Simplest would probably be to just prefer the config over the db settings and disable the webui settings if they are explicitly set in config/env/cli.

Thank you @ErikBjare for the detailed feedback. I completely agree with the proposed precedence order (CLI > Env > Config File > WebUI/API > Defaults), and I have implemented the changes to reflect exactly this logic:

Config File Priority: the configuration file now takes absolute precedence over the database. If a CORS field is explicitly defined in config.toml, the server uses that value and ignores any existing database overrides for that specific field.
for CLI/Env: I verified that cors settings are not currently managed via CLI or Env variables in aw-server-rust. However, the current architecture is future-proof: since the database fallback only triggers for fields missing from the static configuration, any future addition of CLI/Env overrides would naturally take precedence like port and address.

UI Feedback & Safety: To address the concern about "unreliable" config, the Web UI now detects which fields are defined in the config.toml. These fields are marked in the UI as "Fixed in config file" (highlighted in orange) and the inputs are disabled. This ensures the user is aware that the file-level config is in control and cannot be bypassed via the API.

Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction. Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI. We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration. Dependent on: ActivityWatch/aw-server-rust#581 edited according to the last changes

greptile-apps · 2026-04-09T14:38:40Z

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

aw-server/src/endpoints/cors_config.rs

Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction. Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI. We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration. Dependent on: ActivityWatch/aw-server-rust#581 edited according to the last changes

Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction. Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI. We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration. Dependent on: ActivityWatch/aw-webui#795 edited according to the last changes

Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction. Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI. We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration. Dependent on: ActivityWatch/aw-server-rust#581 edited according to the last changes

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 5 times, most recently from 325720e to 587bb8c Compare April 2, 2026 11:47

RaoufGhrissi marked this pull request as ready for review April 2, 2026 11:50

greptile-apps bot reviewed Apr 2, 2026

View reviewed changes

ErikBjare reviewed Apr 2, 2026

View reviewed changes

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 2 times, most recently from 35a5fe8 to 61a534b Compare April 2, 2026 14:50

RaoufGhrissi marked this pull request as draft April 2, 2026 15:05

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from 81acd82 to b585230 Compare April 2, 2026 15:16

RaoufGhrissi commented Apr 2, 2026

View reviewed changes

RaoufGhrissi mentioned this pull request Apr 2, 2026

[IMP] add server settings for CORS ActivityWatch/aw-webui#795

Open

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from b585230 to c44c7df Compare April 2, 2026 15:29

ErikBjare reviewed Apr 2, 2026

View reviewed changes

aw-server/src/config.rs Show resolved Hide resolved

ErikBjare reviewed Apr 2, 2026

View reviewed changes

aw-server/src/endpoints/cors.rs Show resolved Hide resolved

ErikBjare reviewed Apr 2, 2026

View reviewed changes

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from c44c7df to 4b8e893 Compare April 2, 2026 17:07

RaoufGhrissi marked this pull request as ready for review April 2, 2026 17:11

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from 4b8e893 to 102e1f1 Compare April 2, 2026 17:13

greptile-apps bot reviewed Apr 2, 2026

View reviewed changes

aw-server/src/endpoints/cors.rs Show resolved Hide resolved

aw-server/src/endpoints/mod.rs Outdated Show resolved Hide resolved

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 2 times, most recently from 2e6e544 to e3c3227 Compare April 2, 2026 17:26

RaoufGhrissi marked this pull request as draft April 2, 2026 17:45

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 2 times, most recently from 72cdf68 to 51d5a4a Compare April 2, 2026 21:21

RaoufGhrissi marked this pull request as ready for review April 2, 2026 21:22

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from 024a3bf to 8f285a5 Compare April 2, 2026 21:40

RaoufGhrissi marked this pull request as draft April 2, 2026 21:49

RaoufGhrissi marked this pull request as ready for review April 2, 2026 21:49

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 2 times, most recently from 823c6fb to 907e640 Compare April 3, 2026 05:56

RaoufGhrissi requested a review from ErikBjare April 8, 2026 08:39

ErikBjare reviewed Apr 8, 2026

View reviewed changes

RaoufGhrissi force-pushed the security/configure-allowed-origins branch 2 times, most recently from b175b15 to 32b20d7 Compare April 9, 2026 14:24

RaoufGhrissi marked this pull request as draft April 9, 2026 14:33

RaoufGhrissi marked this pull request as ready for review April 9, 2026 14:33

greptile-apps bot reviewed Apr 9, 2026

View reviewed changes

aw-server/src/endpoints/cors_config.rs Show resolved Hide resolved

aw-server/src/endpoints/cors_config.rs Show resolved Hide resolved

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from 32b20d7 to c8695d5 Compare April 9, 2026 15:02

RaoufGhrissi marked this pull request as draft April 9, 2026 15:03

RaoufGhrissi marked this pull request as ready for review April 9, 2026 15:03

RaoufGhrissi marked this pull request as draft April 9, 2026 15:32

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from c8695d5 to 5b85c03 Compare April 9, 2026 15:34

RaoufGhrissi marked this pull request as ready for review April 9, 2026 15:35

RaoufGhrissi force-pushed the security/configure-allowed-origins branch from 5b85c03 to 4b48d81 Compare April 9, 2026 22:08

RaoufGhrissi marked this pull request as draft April 9, 2026 22:10

RaoufGhrissi marked this pull request as ready for review April 9, 2026 22:10

Uh oh!

Conversation

RaoufGhrissi commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ErikBjare Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

RaoufGhrissi Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ErikBjare Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

RaoufGhrissi Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

ErikBjare Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

RaoufGhrissi Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ErikBjare Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

RaoufGhrissi Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Apr 9, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RaoufGhrissi commented Apr 2, 2026 •

edited

Loading

greptile-apps bot commented Apr 2, 2026 •

edited

Loading

RaoufGhrissi Apr 9, 2026 •

edited

Loading